Skip to content

fix(security): harden _blank navigation against reverse-tabnabbing#608

Open
lorenzbaum wants to merge 1 commit intonetbirdio:mainfrom
lorenzbaum:fix/reverse-tabnabbing
Open

fix(security): harden _blank navigation against reverse-tabnabbing#608
lorenzbaum wants to merge 1 commit intonetbirdio:mainfrom
lorenzbaum:fix/reverse-tabnabbing

Conversation

@lorenzbaum
Copy link
Copy Markdown

@lorenzbaum lorenzbaum commented Apr 9, 2026

This PR standardizes secure external navigation across the dashboard.

Changes

  • Centralized hardening for _blank links in shared components (e.g. InlineLink, DropdownMenuItem).
  • Added/normalized rel="noopener noreferrer" for additional link usages and native <a> elements across the codebase.
  • Updated _blank window.open(...) usages to include secure features (noopener,noreferrer).
  • Kept existing behavior unchanged for non-_blank navigation.

Why

Prevents reverse-tabnabbing (window.opener access) and aligns with common security-lint expectations.

Validation

  • Type/lint checks on touched files
  • Manual smoke test of external links and _blank flows

Issue ticket number and link

None

Documentation

Select exactly one:

  • I added/updated documentation for this change
  • Documentation is not needed for this change (explain why)

Docs PR URL (required if "docs added" is checked)

Paste the PR link from https://github.com/netbirdio/docs here:

https://github.com/netbirdio/docs/pull/__

Summary by CodeRabbit

  • Bug Fixes

    • Improved security for external links opened in new tabs by ensuring appropriate rel attributes are applied.
  • Refactor

    • Standardized link behavior across components for consistent new-tab handling; some redundant attributes were also cleaned up.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 9, 2026

📝 Walkthrough

Walkthrough

Adds or centralizes security attributes for external links: components now compute/apply rel="noopener noreferrer" when target="_blank", and multiple pages/modules had rel="noopener noreferrer" added or removed accordingly.

Changes

Cohort / File(s) Summary
Component-level behavior
src/components/DropdownMenu.tsx, src/components/InlineLink.tsx, src/components/SidebarItem.tsx
DropdownMenu and InlineLink compute a memoized safeRel that merges/deduplicates rel tokens and ensures noopener/noreferrer when target="_blank". SidebarItem calls window.open(href, "_blank", "noopener,noreferrer") for _blank targets.
Pages & modules — added rel
src/app/(dashboard)/control-center/page.tsx, src/modules/onboarding/OnboardingEnd.tsx, src/modules/setup-netbird-modal/AndroidTab.tsx, src/modules/setup-netbird-modal/DockerTab.tsx, src/modules/setup-netbird-modal/IOSTab.tsx, src/modules/setup-netbird-modal/MacOSTab.tsx
Added rel="noopener noreferrer" to external/new-tab Link/anchor elements.
UI cleanup
src/components/ui/HelpAndSupportButton.tsx
Removed explicit rel="noopener noreferrer" from DropdownMenuItem links (now handled by DropdownMenu logic).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hop through code, both near and far,
I stitch safe links like a tiny star.
With noopener and noreferrer snug,
New tabs open safe — a rabbit's hug. 🥕✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically summarizes the main security-focused change: hardening _blank navigation against reverse-tabnabbing, which is the central objective of this PR.
Description check ✅ Passed The description comprehensively covers the PR's scope, rationale, changes, and validation approach. It correctly marks documentation as not needed with a brief explanation.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@lorenzbaum lorenzbaum marked this pull request as ready for review April 9, 2026 00:27
Copilot AI review requested due to automatic review settings April 9, 2026 00:27
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens external navigation across the dashboard to prevent reverse-tabnabbing by ensuring _blank links and window.open(...) calls are using noopener/noreferrer protections consistently.

Changes:

  • Added/normalized rel="noopener noreferrer" on various _blank links (including native <a> and next/link usages).
  • Centralized safe rel behavior for shared link-like components (InlineLink, DropdownMenuItem) when target="_blank".
  • Updated _blank window.open(...) usage to include noopener,noreferrer window features.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/modules/setup-netbird-modal/MacOSTab.tsx Adds rel="noopener noreferrer" to external _blank links.
src/modules/setup-netbird-modal/IOSTab.tsx Adds rel="noopener noreferrer" to App Store _blank link.
src/modules/setup-netbird-modal/DockerTab.tsx Adds rel="noopener noreferrer" to Docker docs _blank link.
src/modules/setup-netbird-modal/AndroidTab.tsx Adds rel="noopener noreferrer" to Google Play _blank link.
src/modules/onboarding/OnboardingEnd.tsx Adds rel="noopener noreferrer" to YouTube _blank link.
src/components/ui/HelpAndSupportButton.tsx Removes redundant rel props now handled by DropdownMenuItem.
src/components/SidebarItem.tsx Secures window.open for _blank with noopener,noreferrer.
src/components/InlineLink.tsx Ensures _blank links always include noopener noreferrer (while preserving existing rel tokens).
src/components/DropdownMenu.tsx Ensures _blank dropdown menu links always include noopener noreferrer (while preserving existing rel tokens).
src/app/(dashboard)/control-center/page.tsx Adds rel="noopener noreferrer" to a native <a target="_blank">.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/components/InlineLink.tsx
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/components/InlineLink.tsx (1)

38-46: Consider extracting safeRel normalization into a shared helper.

This logic now exists in both InlineLink and DropdownMenuItem; centralizing it would reduce drift and keep future security updates consistent.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/InlineLink.tsx` around lines 38 - 46, Extract the
rel-normalization logic into a shared helper (e.g., getSafeRel or normalizeRel)
that accepts (rel?: string, target?: string) and returns the normalized rel
string; implement it to split tokens, filter empties, add "noopener" and
"noreferrer" when target === "_blank", de-duplicate and join tokens, then
replace the inline memo in the InlineLink component (safeRel) and the similar
logic in DropdownMenuItem to call this new helper; export the helper from a
common utilities module so both components import and use it, and keep the
memoization in InlineLink (React.useMemo) but use the helper inside it to avoid
behavior changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/components/InlineLink.tsx`:
- Around line 38-46: Extract the rel-normalization logic into a shared helper
(e.g., getSafeRel or normalizeRel) that accepts (rel?: string, target?: string)
and returns the normalized rel string; implement it to split tokens, filter
empties, add "noopener" and "noreferrer" when target === "_blank", de-duplicate
and join tokens, then replace the inline memo in the InlineLink component
(safeRel) and the similar logic in DropdownMenuItem to call this new helper;
export the helper from a common utilities module so both components import and
use it, and keep the memoization in InlineLink (React.useMemo) but use the
helper inside it to avoid behavior changes.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9d2161ea-0d5e-4362-b0f6-9bdf948f7244

📥 Commits

Reviewing files that changed from the base of the PR and between 9701e65 and 3e1d068.

📒 Files selected for processing (10)
  • src/app/(dashboard)/control-center/page.tsx
  • src/components/DropdownMenu.tsx
  • src/components/InlineLink.tsx
  • src/components/SidebarItem.tsx
  • src/components/ui/HelpAndSupportButton.tsx
  • src/modules/onboarding/OnboardingEnd.tsx
  • src/modules/setup-netbird-modal/AndroidTab.tsx
  • src/modules/setup-netbird-modal/DockerTab.tsx
  • src/modules/setup-netbird-modal/IOSTab.tsx
  • src/modules/setup-netbird-modal/MacOSTab.tsx
💤 Files with no reviewable changes (1)
  • src/components/ui/HelpAndSupportButton.tsx

@lorenzbaum lorenzbaum force-pushed the fix/reverse-tabnabbing branch from 3e1d068 to b502e4d Compare April 9, 2026 00:33
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/components/DropdownMenu.tsx (1)

114-122: Consider centralizing safeRel generation to avoid security-policy drift.

This logic is now duplicated with src/components/InlineLink.tsx:37-56. Extracting a small shared helper (e.g., buildSafeRel(target, rel)) would keep _blank hardening consistent across components.

♻️ Proposed refactor (shared helper + local usage)
+// e.g. src/utils/linkSecurity.ts
+export function buildSafeRel(target?: string, rel?: string) {
+  if (target !== "_blank") return rel;
+
+  const tokens = new Set((rel ?? "").split(/\s+/).filter(Boolean));
+  tokens.add("noopener");
+  tokens.add("noreferrer");
+
+  return Array.from(tokens).join(" ");
+}
-    const safeRel = React.useMemo(() => {
-      if (target !== "_blank") return rel;
-
-      const tokens = new Set((rel ?? "").split(/\s+/).filter(Boolean));
-      tokens.add("noopener");
-      tokens.add("noreferrer");
-
-      return Array.from(tokens).join(" ");
-    }, [target, rel]);
+    const safeRel = React.useMemo(() => buildSafeRel(target, rel), [target, rel]);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/DropdownMenu.tsx` around lines 114 - 122, The safeRel
construction in DropdownMenu (the React.useMemo that computes safeRel) is
duplicated with InlineLink; extract a small shared helper function like
buildSafeRel(target, rel) that centralizes adding "noopener" and "noreferrer"
when target === "_blank" and returns the joined string, then replace the inline
useMemo in DropdownMenu (and the logic in InlineLink) to call
buildSafeRel(target, rel); update imports where needed and ensure both
components use the same helper to avoid security-policy drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/components/DropdownMenu.tsx`:
- Around line 114-122: The safeRel construction in DropdownMenu (the
React.useMemo that computes safeRel) is duplicated with InlineLink; extract a
small shared helper function like buildSafeRel(target, rel) that centralizes
adding "noopener" and "noreferrer" when target === "_blank" and returns the
joined string, then replace the inline useMemo in DropdownMenu (and the logic in
InlineLink) to call buildSafeRel(target, rel); update imports where needed and
ensure both components use the same helper to avoid security-policy drift.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 36636fc1-4a46-4b77-bffe-ceef12c656d8

📥 Commits

Reviewing files that changed from the base of the PR and between 3e1d068 and b502e4d.

📒 Files selected for processing (10)
  • src/app/(dashboard)/control-center/page.tsx
  • src/components/DropdownMenu.tsx
  • src/components/InlineLink.tsx
  • src/components/SidebarItem.tsx
  • src/components/ui/HelpAndSupportButton.tsx
  • src/modules/onboarding/OnboardingEnd.tsx
  • src/modules/setup-netbird-modal/AndroidTab.tsx
  • src/modules/setup-netbird-modal/DockerTab.tsx
  • src/modules/setup-netbird-modal/IOSTab.tsx
  • src/modules/setup-netbird-modal/MacOSTab.tsx
💤 Files with no reviewable changes (1)
  • src/components/ui/HelpAndSupportButton.tsx
✅ Files skipped from review due to trivial changes (7)
  • src/modules/setup-netbird-modal/AndroidTab.tsx
  • src/modules/setup-netbird-modal/DockerTab.tsx
  • src/modules/onboarding/OnboardingEnd.tsx
  • src/app/(dashboard)/control-center/page.tsx
  • src/modules/setup-netbird-modal/MacOSTab.tsx
  • src/modules/setup-netbird-modal/IOSTab.tsx
  • src/components/SidebarItem.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/components/InlineLink.tsx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants